Skip to content

Conversation

tbaederr
Copy link
Contributor

@tbaederr tbaederr commented Feb 4, 2025

See the attached test case.

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Feb 4, 2025
@llvmbot
Copy link
Member

llvmbot commented Feb 4, 2025

@llvm/pr-subscribers-clang

Author: Timm Baeder (tbaederr)

Changes

See the attached test case.


Full diff: https://github.com/llvm/llvm-project/pull/125727.diff

3 Files Affected:

  • (modified) clang/lib/AST/ByteCode/Function.h (+5)
  • (modified) clang/lib/AST/ByteCode/InterpFrame.cpp (+1-1)
  • (modified) clang/test/AST/ByteCode/lifetimes.cpp (+23)
diff --git a/clang/lib/AST/ByteCode/Function.h b/clang/lib/AST/ByteCode/Function.h
index 2d3421e5e61295..e17183eef9eac6 100644
--- a/clang/lib/AST/ByteCode/Function.h
+++ b/clang/lib/AST/ByteCode/Function.h
@@ -51,6 +51,11 @@ class Scope final {
     return llvm::make_range(Descriptors.begin(), Descriptors.end());
   }
 
+  llvm::iterator_range<LocalVectorTy::const_reverse_iterator>
+  locals_reverse() const {
+    return llvm::reverse(Descriptors);
+  }
+
 private:
   /// Object descriptors in this block.
   LocalVectorTy Descriptors;
diff --git a/clang/lib/AST/ByteCode/InterpFrame.cpp b/clang/lib/AST/ByteCode/InterpFrame.cpp
index 89fc7a4515d641..c383b2bc7f95ce 100644
--- a/clang/lib/AST/ByteCode/InterpFrame.cpp
+++ b/clang/lib/AST/ByteCode/InterpFrame.cpp
@@ -99,7 +99,7 @@ void InterpFrame::initScope(unsigned Idx) {
 }
 
 void InterpFrame::destroy(unsigned Idx) {
-  for (auto &Local : Func->getScope(Idx).locals()) {
+  for (auto &Local : Func->getScope(Idx).locals_reverse()) {
     S.deallocate(localBlock(Local.Offset));
   }
 }
diff --git a/clang/test/AST/ByteCode/lifetimes.cpp b/clang/test/AST/ByteCode/lifetimes.cpp
index 43039d0c766e9e..318bc205c76724 100644
--- a/clang/test/AST/ByteCode/lifetimes.cpp
+++ b/clang/test/AST/ByteCode/lifetimes.cpp
@@ -68,3 +68,26 @@ namespace PrimitiveMoveFn {
     const float &x = y;
   }
 }
+
+namespace LocalDestroy {
+  /// This is reduced from a libc++ test case.
+  /// The local f.TI.copied points to the local variable Copied, and we used to
+  /// destroy Copied before f, causing problems later on when a DeadBlock had a
+  /// pointer pointing to it that was already destroyed.
+  struct TrackInitialization {
+    bool *copied_;
+  };
+  struct TrackingPred : TrackInitialization {
+    constexpr TrackingPred(bool *copied) : TrackInitialization(copied) {}
+  };
+  struct F {
+    const TrackingPred &TI;
+  };
+  constexpr int f() {
+    bool Copied = false;
+    TrackingPred TI(&Copied);
+    F f{TI};
+    return 1;
+  }
+  static_assert(f() == 1);
+}

@tbaederr tbaederr merged commit 16c721f into llvm:main Feb 5, 2025
8 checks passed
Icohedron pushed a commit to Icohedron/llvm-project that referenced this pull request Feb 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants